Skip to content

Fix doc comments in Process.fs#2885

Open
Numpsy wants to merge 1 commit into
fsprojects:masterfrom
Numpsy:doc_comments
Open

Fix doc comments in Process.fs#2885
Numpsy wants to merge 1 commit into
fsprojects:masterfrom
Numpsy:doc_comments

Conversation

@Numpsy
Copy link
Copy Markdown
Contributor

@Numpsy Numpsy commented Nov 30, 2025

The Exec functions have a parameter called 'dir' but the doc comments have 'directory', so this just makes them consistent

(Just noticed a warning in Rider when looking at #2880 and though they should be consistent)

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Nov 30, 2025

Test Results

   12 files  ±0     12 suites  ±0   36m 34s ⏱️ + 2m 32s
  451 tests ±0    450 ✅ ±0  1 💤 ±0  0 ❌ ±0 
1 287 runs  ±0  1 284 ✅ ±0  3 💤 ±0  0 ❌ ±0 

Results for commit 77970c5. ± Comparison against base commit e7d3021.

♻️ This comment has been updated with latest results.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates XML doc comments in Fake.Core.Process’s Shell API to match the actual optional parameter name (dir) and fixes a small grammar issue in the summary text.

Changes:

  • Update <param> name from directory to dir for Shell.Exec and Shell.AsyncExec.
  • Fix “it’s completion” → “its completion” in the Shell.Exec summary.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 933 to 934
/// <param name="cmd">The command which should be run in elevated context.</param>
/// <param name="args">The process arguments (optional).</param>
Comment on lines 943 to 944
/// <param name="cmd">The command which should be run in elevated context.</param>
/// <param name="args">The process arguments (optional).</param>
The Exec functions have a parameter called 'dir' but the doc comments have 'directory', so this just makes them consistent
@xperiandri
Copy link
Copy Markdown
Collaborator

Are there any relevant comments from Copilot?

@Numpsy
Copy link
Copy Markdown
Contributor Author

Numpsy commented May 12, 2026

I wasn't looking at anything other than the param names before, but looking at the code now the 'async' function doesn't look very async? (actually it looks the same as the implementation of the not-async version)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants